-
Notifications
You must be signed in to change notification settings - Fork 487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize tree walk to avoid too depth function call stack. #326
Conversation
Codecov Report
@@ Coverage Diff @@
## master #326 +/- ##
==========================================
+ Coverage 83.1% 83.14% +0.04%
==========================================
Files 142 142
Lines 3172 3181 +9
Branches 654 654
==========================================
+ Hits 2636 2645 +9
Misses 429 429
Partials 107 107
Continue to review full report at Codecov.
|
3d691f8
to
1f463dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An iterative approach is definitely be more efficient than the recursive approach (especially in JS), thanks!
Looks like there might be an issue with the depth. Can you take a look at the comments? Would you mind adding a test-case that checks for this?
Signed-off-by: ruben.vargas <[email protected]>
1f463dc
to
ae3cde4
Compare
Signed-off-by: ruben.vargas <[email protected]>
ae3cde4
to
8b30e0f
Compare
Ready for another review :) |
Awesome work! Thanks so much for making these changes! 🎉 |
…cing#326) * Optimize tree walk Signed-off-by: ruben.vargas <[email protected]> * Add test to treeNode walk function for check depth calculation Signed-off-by: ruben.vargas <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
Which problem is this PR solving?
Short description of the changes
I did some tests and it seems to behave better with an iterative walk algorithm. This partially solves the problem, because the render of too many components is slow. We could probably need to impose a limit on the depth (But at least now is not freezing anymore)